Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

K2 plugin migration #321

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

K2 plugin migration #321

wants to merge 2 commits into from

Conversation

akozlova
Copy link

@akozlova akozlova commented Oct 1, 2024

Evident usages of K1 api (resolve) are migrated

build.gradle.kts Outdated
Comment on lines 97 to 119
//tasks.named<RunIdeTask>("runIde") {
// jvmArgumentProviders += CommandLineArgumentProvider {
// listOf("-Didea.kotlin.plugin.use.k2=true")
// }
//}

//tasks.test {
// jvmArgumentProviders += CommandLineArgumentProvider {
// listOf("-Didea.kotlin.plugin.use.k2=true")
// }
//}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should not commit out-commented code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without vm options, tests and debug IDE would be run against K1 mode. It's ok for 242 and below but for the 243+, it should be changed to something else. It's better to separate migration of the plugin and migration of the test infra indeed. Hopefully, it would be easy for the maintainers to find corresponding options without the comments.

@@ -21,51 +21,6 @@ class SpecTests : LightJavaCodeInsightFixtureTestCase() {
return false
}

fun testIsContainedInSpecFunSpec() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of merging `master´, rebase to keep the PR's history clean of merge commits for more convenient review.

- starting from 242, the plugin works over AnalysisAPI which hides all details about kotlin engine used inside kotlin plugin itself
@akozlova akozlova reopened this Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants